-
Notifications
You must be signed in to change notification settings - Fork 1
Add variable font metadata #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ed17ef1 to
6f276fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- We need axes min/default/max per font, as they vary. Not sure why Google stores global min/default/max for each axis.
- We can reduce the axes registry to an object
"TAG": "Display name"precisionis nice but looks like we can drop it too since we don't have access to that with local fonts, so we'd need to implement a general-purpose heuristic to determine precision either way (font files don't carry this info)
vladmoroz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's also sort the axes in the font metadata same way we are going to do it in the app
|
Ok will adjust |
output/font-chunks/fonts.json
Outdated
| "Zilla Slab Highlight": {"x":166,"y":768,"w":225,"ch":16,"s":["400","700"]} | ||
| "ABeeZee": {"x":0,"y":0,"w":168,"ch":0,"s":{"v":["400","400i"]}}, | ||
| "ADLaM Display": {"x":157,"y":64,"w":219,"ch":0,"s":{"v":["400"]}}, | ||
| "AR One Sans": {"x":0,"y":704,"w":244,"ch":0,"s":{"v":["400","500","600","700"],"a":[{"tag":"wght","min":400,"max":700},{"tag":"ARRR","min":10,"max":60}]}}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason to change styles "s" key to "v" inside "s"? prefer flatter objects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok will bring them out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need the "s" → "v" rename too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah right might as well revert that, will do!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
oh one more before we can merge, we still need |
|
I'll take a look |
🤔 |
looks good!
huh would we have to keep track of it manually when we regenerate
that's fine, we'll construct them manually at runtime when a font is requested using the data we have here. they describe variable font instances but Google Fonts doesn't seem to have a concept of variable instances at all. |
No it's automatic |
axes.jsonwhich has variable font axes definitionsaxesto metadata.jsonwe'll want to copy
fonts.json,axis.json, and the updatedfont-chunk.avifs into paper